feat(api): Track 3.3 — GET /api/analysis-runs/{run_id}/source-audio - #40
Conversation
Third of four pieces from Track 3. Adds a stable, round-trip-saving
route for re-serving the original ingested audio for a run, so
clients (Phase 2 reruns, external consumers) don't have to fetch the
snapshot first to learn the artifact id.
The same bytes are reachable today via
GET /api/analysis-runs/{run_id}/artifacts/{artifact_id} (source audio
is stored as an artifact with kind='source_audio'). This new route is
an ergonomic shortcut, not a new capability:
- Stable URL (doesn't change per run; the artifact id does)
- Caller doesn't need to fetch the snapshot first
- Identical bytes, identical Content-Disposition and MIME
Auth model:
- Owner-only via _resolve_route_user_context + runtime.get_run(
owner_user_id=...). Standard read pattern.
- Does NOT honor X-Admin-Key. Re-serving another user's audio has a
stronger privacy posture than the cross-user delete admin bypass
(Track 3.2). The review scoped admin to operations, not reads.
- Non-owner request returns RUN_NOT_FOUND, matching the existing
pattern that does not disclose whether a run exists.
Error codes:
- RUN_NOT_FOUND (404) — run missing or not owned.
- SOURCE_AUDIO_NOT_FOUND (404) — defensive; would indicate the run
row exists but source_artifact_id is null/dangling. Distinct from
RUN_NOT_FOUND because the recovery path differs (server-side
corruption vs ownership/typo).
- SOURCE_AUDIO_FILE_MISSING (404) — artifact row intact but bytes
gone from disk. Distinct from SOURCE_AUDIO_NOT_FOUND because the
recovery path differs (re-ingest vs operator look).
Files:
- apps/backend/server.py — new route between the existing artifact-
fetch route and the spectral-enhancements route. ~85 lines.
- apps/backend/tests/test_server.py — 5 new tests in
GetRunSourceAudioRouteTests covering: happy path with content-
disposition; non-owner returns RUN_NOT_FOUND; unknown run returns
RUN_NOT_FOUND; missing source artifact returns SOURCE_AUDIO_NOT_FOUND;
missing file on disk returns SOURCE_AUDIO_FILE_MISSING.
- apps/backend/ARCHITECTURE.md — documents the new route.
Out of scope (deferred):
- Range requests / partial content for streaming. The FileResponse
doesn't currently support Range; sufficient for the audio sizes
ASA handles (100 MiB cap).
- Frontend UI that uses this endpoint. Phase 2 reruns are the
intended consumer; UI wiring is a follow-up.
https://claude.ai/code/session_01Uh2eNT1oqjixcox8UX3BTg
slittycode
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
Adds GET /api/analysis-runs/{run_id}/source-audio as an ergonomic stable-URL shortcut over the existing GET .../artifacts/{artifact_id} route. The route is a thin shell over get_source_artifact + resolve_artifact_local_path, follows the existing ownership-check pattern exactly, and comes with 5 tests covering all documented error branches. No Phase 1 data touched, no schema changes, no DSP.
The venv wasn't bootstrapped in this environment so I reviewed the tests statically; CI will catch execution failures.
Findings
Should fix:
SOURCE_AUDIO_NOT_FOUND and SOURCE_AUDIO_FILE_MISSING error bodies omit requestId, retryable, and diagnostics. CLAUDE.md states these are always required in error envelopes. The existing _run_not_found_response, ARTIFACT_NOT_FOUND, ARTIFACT_FILE_MISSING, and AUTHENTICATION_REQUIRED responses all have the same omission — so the new code is consistent with its neighbors — but that doesn't make it compliant. If the codebase is ever brought into full contract compliance on the run-management routes, these two new codes will need the same fix.
Worth considering:
get_source_artifact re-queries analysis_runs independently from get_run() (server.py:2321). In a concurrent delete + fetch race, get_source_artifact raises KeyError("Unknown run …") which the except KeyError: at server.py:2322 catches and maps to SOURCE_AUDIO_NOT_FOUND rather than RUN_NOT_FOUND. Both are 404s so this is a misclassification, not a data hazard. Practically unreachable but worth naming if the error codes are ever surfaced to operators for triage.
test_unknown_run_returns_404 doesn't patch SONIC_ANALYZER_RUNTIME_PROFILE to hosted the way the other four tests in GetRunSourceAudioRouteTests do. Functionally correct (local profile assigns user_id="local-dev" and the run genuinely doesn't exist), just inconsistent with the rest of the class setup.
Test results
5 new tests in GetRunSourceAudioRouteTests. Static analysis: all logic branches covered — happy path, non-owner, unknown run, missing artifact row, missing file on disk. No execution environment available; CI is the gate.
Phase boundary check
Clean. No Phase 1 fields read, mutated, or re-derived. No changes to analyze.py, any analyze_*.py module, Phase 2 prompts, or types.ts.
Generated by Claude Code
…onsistency Addresses PR #40 review: 1. Should fix: SOURCE_AUDIO_NOT_FOUND and SOURCE_AUDIO_FILE_MISSING error envelopes now carry the 'retryable' boolean per the CLAUDE.md contract. Both are False — corrupted run state and missing-on-disk bytes don't recover from a naive retry; operator action is required. The contract also calls for requestId and diagnostics; those are omitted across the existing run-management routes (ARTIFACT_NOT_FOUND, ARTIFACT_FILE_MISSING, AUTHENTICATION_REQUIRED), and bringing the whole surface into full compliance is out of scope for this PR. Adding retryable here keeps the new code from drifting further. 2. Worth considering: test_unknown_run_returns_404 didn't patch SONIC_ANALYZER_RUNTIME_PROFILE the way the other four tests in the class do. Functionally correct (local profile auto-assigns local-dev) but inconsistent. Now patches to hosted for setup parity. 3. Not fixed (per reviewer's 'practically unreachable'): the delete/fetch race that maps to SOURCE_AUDIO_NOT_FOUND instead of RUN_NOT_FOUND. Disambiguation requires either string-matching the KeyError message (fragile) or adding owner_user_id threading to get_source_artifact. Both are real cost for an unreachable case. The existing SOURCE_AUDIO_NOT_FOUND / SOURCE_AUDIO_FILE_MISSING route tests now assert the retryable field, so a future regression (removing it) would fail. https://claude.ai/code/session_01Uh2eNT1oqjixcox8UX3BTg
The Frontend CI on the previous commit failed after 1m 53s, which is
too short for smoke tests to have executed — npm ci + playwright
install --with-deps chromium typically take 60-90 s combined before
'npm run verify' even starts.
Local checks all pass: npm run lint (clean), npm run test:unit
(398/398), npm run build (clean). Backend CI on the same workflow
passed at 4m 21s.
My local apt-get failed when installing Playwright system deps
('ppa:ondrej/php repository no longer signed'), suggesting a similar
transient registry/apt blip caused the CI failure. Empty commit
retriggers CI to confirm.
Smoke tests stub all backend routes via page.route() in their specs,
so the new GET /api/analysis-runs/{run_id}/source-audio route this
PR adds cannot affect them.
https://claude.ai/code/session_01Uh2eNT1oqjixcox8UX3BTg
Fourth and final piece of Track 3 from the merged external-repo review.
Additive over the existing 8-state stage status — does not change or
remove anything.
What's new:
- apps/backend/stage_status.py (new) — pure module with the canonical
8 -> 5 mapping table, the to_public_status() helper, and
PUBLIC_STATUS_VALUES exported for type generation / docs.
Mapping (internal -> public):
queued -> queued
running -> running
blocked -> queued (transient scheduling state)
ready -> queued (scheduled but not yet running)
completed -> completed
failed -> failed
interrupted -> interrupted
not_requested -> null (stage not in pipeline)
- apps/backend/server_phase1.py — _normalize_run_snapshot now also
annotates each stage with publicStatus via the new
_annotate_public_status helper. The original status field is
preserved untouched.
- apps/backend/tests/test_stage_status.py (new, 16 tests) — pins
every cell of the mapping, the public-value set, defensive cases
(None, unknown, empty, case-sensitive), and a completeness check
that fails if the set of internal states drifts without an explicit
mapping update.
- apps/backend/tests/test_server.py (+1 test) — integration test
that GET /api/analysis-runs/{run_id} returns publicStatus on all
three stages with the correct values for a fresh run.
- apps/ui/src/types/backend.ts — new PublicStageStatus type (the
5-value union). publicStatus: PublicStageStatus | null added to
MeasurementStageSnapshot, PitchNoteTranslationStageSnapshot,
InterpretationStageSnapshot. attempt summaries unchanged
(publicStatus is only on top-level stages).
- apps/ui/src/services/analysisRunsClient.ts — new
parsePublicStageStatus helper + PUBLIC_STAGE_STATUSES set; threaded
into the three stage parsers. The helper accepts null and missing
values without throwing so legacy snapshots that pre-date the
field continue to deserialize.
- docs/adr/0001-phase1-json-schema-v1.md — documents the additive
field, the mapping table, and the rationale (internal scheduling
states are scheduling concerns the client doesn't need to act on).
Out of scope (deferred):
- Removing publicStatus duplication from the internal status field
later. The additive approach was the explicit choice; collapsing
to one field would be a breaking change requiring a v2 schema bump.
- Frontend UI that reads publicStatus. Existing UI continues to use
the 8-state status field; publicStatus is the additive surface for
future external consumers and any new UI views that prefer the
smaller vocabulary.
This closes Track 3 (all four pieces shipped):
- 3.1 URL ingestion (#36, merged)
- 3.2 X-Admin-Key on DELETE (#37, merged)
- 3.3 GET /source-audio re-serve (#40, merged)
- 3.4 publicStatus additive (this PR)
https://claude.ai/code/session_01Uh2eNT1oqjixcox8UX3BTg
Co-authored-by: Claude <noreply@anthropic.com>
Third of four pieces from Track 3 of the merged external-repo review. Adds a stable, round-trip-saving route for re-serving the original ingested audio for a run.
Why a new route when the bytes are already reachable?
The same bytes are reachable today via
GET /api/analysis-runs/{run_id}/artifacts/{artifact_id}— source audio is stored as an artifact withkind="source_audio". So this is an ergonomic shortcut, not a net-new capability. The win is:/source-audiodoesn't change per run; the artifact id does./api/analysis-runs/{id}/source-audiodirectly into a Max patch, a REAPER script, or a CLI fetch — no inspection step needed.Identical bytes, identical
Content-Dispositionand MIME on the wire.Auth model
_resolve_route_user_context+runtime.get_run(owner_user_id=…).X-Admin-Key. Re-serving another user's audio has a stronger privacy posture than the cross-user delete admin bypass added in Track 3.2. Reading user content cross-tenant is more sensitive than reversibly purging it.RUN_NOT_FOUND(does not disclose whether the run exists), matching the existing pattern.Error envelope
audio/<original-mime>RUN_NOT_FOUNDsource_artifact_idis null/dangling (defensive)SOURCE_AUDIO_NOT_FOUNDSOURCE_AUDIO_FILE_MISSINGThe two "source missing" variants are distinct because the recovery paths differ —
SOURCE_AUDIO_NOT_FOUNDindicates server-side corruption an operator should investigate;SOURCE_AUDIO_FILE_MISSINGmeans the metadata is intact and re-ingestion is the fix.Files
apps/backend/server.pyget_run_source_audioroute handler, placed next to the existingget_run_artifacthandler. ~85 lines.apps/backend/tests/test_server.pyGetRunSourceAudioRouteTests: happy path with Content-Disposition + MIME; non-owner →RUN_NOT_FOUND; unknown run →RUN_NOT_FOUND; missing source artifact →SOURCE_AUDIO_NOT_FOUND; missing file on disk →SOURCE_AUDIO_FILE_MISSING.apps/backend/ARCHITECTURE.mdVerification
Route tests use the existing
AnalysisRuntime+patch.object(server, "get_analysis_runtime", …)pattern. CI will exercise them. No pure-logic module to unit-test separately — the route is a thin shell over existing runtime helpers (get_source_artifact,resolve_artifact_local_path).What this PR does NOT do
FileResponseserves the whole file; ASA's 100 MiB cap keeps that bounded. Range support could be added later if a UI playback feature needs it.Track 3 sequencing
X-Admin-KeyonDELETE(feat(api): Track 3.2 — X-Admin-Key gates cross-user DELETE on /api/analysis-runs #37, merged)GET …/source-audiore-servehttps://claude.ai/code/session_01Uh2eNT1oqjixcox8UX3BTg
Generated by Claude Code